Refactor/clang tidy cleanup #1340

SteveBronder · 2019-09-05T20:05:26Z

Edit: Something weird is going on with the Jenkinsbot so I move the branch over to the main repo

Summary

Running clang-tidy with the checks below. This refactor is only aesthetic and does not change how any of the code is executed.

readability-braces-around-statements:

Adds braces around conditionals and ifs

modernize-use-using:
everywhere clang-tidy can replace a typedef with a using it will aka
typedef double A; becomes using A = double;

modernize-use-nullptr:
using nullptr instead of NULL

The original PR also had a cleanup to return a braced init list but after looking at the I didn't like it

Tests

refactor is only aesthetic so no new tests

Checklist

Math issue Update internals to use more modern c++ #1308
Copyright holder: Steve Bronder

The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
- unit tests pass (to run, use: ./runTests.py test/unit)
- header checks pass, (make test-headers)
- docs build, (make doxygen)
- code passes the built in C++ standards checks (make cpplint)
the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested

…stable/2017-11-14)

…h into refactor/clang-tidy-cleanup

…xp1~20180509124008.99 (branches/release_50)

stan-buildbot · 2019-09-06T01:35:31Z

(stat_comp_benchmarks/benchmarks/gp_pois_regr/gp_pois_regr.stan, 1.0)
(stat_comp_benchmarks/benchmarks/low_dim_corr_gauss/low_dim_corr_gauss.stan, 0.99)
(stat_comp_benchmarks/benchmarks/irt_2pl/irt_2pl.stan, 1.0)
(stat_comp_benchmarks/benchmarks/pkpd/one_comp_mm_elim_abs.stan, 0.99)
(stat_comp_benchmarks/benchmarks/eight_schools/eight_schools.stan, 1.01)
(stat_comp_benchmarks/benchmarks/gp_regr/gp_regr.stan, 0.99)
(stat_comp_benchmarks/benchmarks/arK/arK.stan, 1.0)
(performance.compilation, 1.02)
(stat_comp_benchmarks/benchmarks/low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan, 1.01)
(stat_comp_benchmarks/benchmarks/low_dim_gauss_mix/low_dim_gauss_mix.stan, 1.0)
(stat_comp_benchmarks/benchmarks/sir/sir.stan, 0.98)
(stat_comp_benchmarks/benchmarks/pkpd/sim_one_comp_mm_elim_abs.stan, 1.01)
(stat_comp_benchmarks/benchmarks/garch/garch.stan, 1.01)
(stat_comp_benchmarks/benchmarks/gp_regr/gen_gp_data.stan, 1.0)
(stat_comp_benchmarks/benchmarks/arma/arma.stan, 0.99)
Result: 0.99978557888
Commit hash: 3e9d7fe

rok-cesnovar

Looks great, the changes for braces are all fine.

There are a few files where we are changing some typedefs to using but some are left as typedefs. For consistency I think we should change the rest to using also. No need to change all the typedefs, I just feel that mixing both in the same file should be avoided.

rok-cesnovar · 2019-09-07T09:06:27Z

stan/math/fwd/mat/fun/typedefs.hpp

@@ -10,18 +10,18 @@ namespace math {

 typedef Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>::Index size_type;


Can we do this one manually so its consistent?

rok-cesnovar · 2019-09-07T09:06:34Z

stan/math/fwd/mat/fun/typedefs.hpp

@@ -10,18 +10,18 @@ namespace math {

 typedef Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>::Index size_type;

-typedef Eigen::Matrix<fvar<double>, Eigen::Dynamic, Eigen::Dynamic> matrix_fd;
+using matrix_fd = Eigen::Matrix<fvar<double>, Eigen::Dynamic, Eigen::Dynamic>;

 typedef Eigen::Matrix<fvar<fvar<double> >, Eigen::Dynamic, Eigen::Dynamic>


This one too.

rok-cesnovar · 2019-09-07T09:10:27Z

stan/math/mix/mat/fun/typedefs.hpp

@@ -9,18 +9,18 @@
 namespace stan {
 namespace math {

-typedef Eigen::Matrix<fvar<var>, Eigen::Dynamic, Eigen::Dynamic> matrix_fv;
+using matrix_fv = Eigen::Matrix<fvar<var>, Eigen::Dynamic, Eigen::Dynamic>;

 typedef Eigen::Matrix<fvar<fvar<var> >, Eigen::Dynamic, Eigen::Dynamic>


For consistency please convert this one to using also.

rok-cesnovar · 2019-09-07T09:23:24Z

stan/math/prim/arr/functor/coupled_ode_observer.hpp

@@ -28,7 +28,7 @@ namespace math {
 */
 template <typename F, typename T1, typename T2, typename T_t0, typename T_ts>
 struct coupled_ode_observer {
-  typedef return_type_t<T1, T2, T_t0, T_ts> return_t;
+  using return_t = return_type_t<T1, T2, T_t0, T_ts>;

  typedef operands_and_partials<std::vector<T1>, std::vector<T2>, T_t0, T_ts>


I am just going to leave "using" for further comments where we can manually replace typdef.

rok-cesnovar · 2019-09-07T09:29:21Z

stan/math/prim/mat/fun/log_mix.hpp

    const T_theta& theta,
-    const std::vector<Eigen::Matrix<T_lam, R, C> >& lambda) {
+    const std::vector<Eigen::Matrix<T_lam, R, C>>& lambda) {
  static const char* function = "log_mix";
  typedef typename stan::partials_return_type<


rok-cesnovar · 2019-09-07T10:27:04Z

stan/math/prim/scal/prob/exp_mod_normal_lpdf.hpp

@@ -24,8 +24,9 @@ return_type_t<T_y, T_loc, T_scale, T_inv_scale> exp_mod_normal_lpdf(
  typedef partials_return_type_t<T_y, T_loc, T_scale, T_inv_scale>


using line 23

rok-cesnovar · 2019-09-07T10:37:13Z

stan/math/prim/scal/prob/pareto_type_2_cdf.hpp

@@ -21,8 +21,9 @@ return_type_t<T_y, T_loc, T_scale, T_shape> pareto_type_2_cdf(
  typedef partials_return_type_t<T_y, T_loc, T_scale, T_shape>


rok-cesnovar · 2019-09-07T10:37:32Z

stan/math/prim/scal/prob/pareto_type_2_lccdf.hpp

@@ -21,8 +21,9 @@ return_type_t<T_y, T_loc, T_scale, T_shape> pareto_type_2_lccdf(
  typedef partials_return_type_t<T_y, T_loc, T_scale, T_shape>


rok-cesnovar · 2019-09-07T10:37:49Z

stan/math/prim/scal/prob/pareto_type_2_lcdf.hpp

@@ -22,8 +22,9 @@ return_type_t<T_y, T_loc, T_scale, T_shape> pareto_type_2_lcdf(
  typedef partials_return_type_t<T_y, T_loc, T_scale, T_shape>


rok-cesnovar · 2019-09-07T10:40:53Z

stan/math/rev/mat/fun/typedefs.hpp

@@ -14,37 +14,37 @@ typedef Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>::Index size_type;
 * The type of a matrix holding <code>var</code>


using at line 11

…xp1~20180509124008.99 (branches/release_50)

SteveBronder · 2019-09-08T16:40:05Z

@rok-cesnovar thanks for the review! I think I cleaned up all the rest of the typedefs. I left the ones in the MPI code that are used in a macro because I was worried about touching those. But everywhere else should be good to go

…gs/RELEASE_500/final)

stan-buildbot · 2019-09-09T00:15:39Z

(stat_comp_benchmarks/benchmarks/gp_pois_regr/gp_pois_regr.stan, 1.01)
(stat_comp_benchmarks/benchmarks/low_dim_corr_gauss/low_dim_corr_gauss.stan, 0.97)
(stat_comp_benchmarks/benchmarks/irt_2pl/irt_2pl.stan, 1.0)
(stat_comp_benchmarks/benchmarks/pkpd/one_comp_mm_elim_abs.stan, 1.02)
(stat_comp_benchmarks/benchmarks/eight_schools/eight_schools.stan, 1.02)
(stat_comp_benchmarks/benchmarks/gp_regr/gp_regr.stan, 0.96)
(stat_comp_benchmarks/benchmarks/arK/arK.stan, 1.0)
(performance.compilation, 1.02)
(stat_comp_benchmarks/benchmarks/low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan, 1.0)
(stat_comp_benchmarks/benchmarks/low_dim_gauss_mix/low_dim_gauss_mix.stan, 1.0)
(stat_comp_benchmarks/benchmarks/sir/sir.stan, 1.05)
(stat_comp_benchmarks/benchmarks/pkpd/sim_one_comp_mm_elim_abs.stan, 1.0)
(stat_comp_benchmarks/benchmarks/garch/garch.stan, 0.99)
(stat_comp_benchmarks/benchmarks/gp_regr/gen_gp_data.stan, 0.97)
(stat_comp_benchmarks/benchmarks/arma/arma.stan, 0.92)
Result: 0.99587011727
Commit hash: 028ab67

rok-cesnovar

LGTM. Agreed on leaving the ones you are not sure about. Thanks!

SteveBronder and others added 10 commits September 5, 2019 10:42

Add braces to everything

5cff178

Add braces to everything

9945e59

replace typedef with using and add nullptr

80c2961

clang-format

b2410f3

run clang-format manually

03dfdde

[Jenkins] auto-formatting by clang-format version 6.0.0 (tags/google/…

6a762ab

…stable/2017-11-14)

kick jenkins

d2c5e37

Merge branch 'refactor/clang-tidy-cleanup' of github.com:stan-dev/mat…

1b0eec0

…h into refactor/clang-tidy-cleanup

[Jenkins] auto-formatting by clang-format version 5.0.2-svn328729-1~e…

e6c269b

…xp1~20180509124008.99 (branches/release_50)

merge to develop

3e9d7fe

SteveBronder requested a review from rok-cesnovar September 6, 2019 07:23

rok-cesnovar self-assigned this Sep 6, 2019

rok-cesnovar requested changes Sep 7, 2019

View reviewed changes

SteveBronder and others added 3 commits September 8, 2019 18:54

merge to develop

1d5ad7e

fix rest of typedefs and merge to develop

0822147

[Jenkins] auto-formatting by clang-format version 5.0.2-svn328729-1~e…

0a24ccc

…xp1~20180509124008.99 (branches/release_50)

SteveBronder and others added 2 commits September 8, 2019 23:07

fix usings

8094ba1

[Jenkins] auto-formatting by clang-format version 5.0.0-3~16.04.1 (ta…

028ab67

…gs/RELEASE_500/final)

rok-cesnovar approved these changes Sep 9, 2019

View reviewed changes

rok-cesnovar merged commit d4e9332 into develop Sep 9, 2019

rok-cesnovar mentioned this pull request Sep 9, 2019

Bugfix/issue 1250 lgamma #1255

Merged

5 tasks

serban-nicusor-toptal added this to the 3.0.0 milestone Oct 18, 2019

SteveBronder deleted the refactor/clang-tidy-cleanup branch December 30, 2019 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/clang tidy cleanup #1340

Refactor/clang tidy cleanup #1340

SteveBronder commented Sep 5, 2019

stan-buildbot commented Sep 6, 2019

rok-cesnovar left a comment

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

rok-cesnovar Sep 7, 2019

SteveBronder commented Sep 8, 2019

stan-buildbot commented Sep 9, 2019

rok-cesnovar left a comment

		@@ -10,18 +10,18 @@ namespace math {

		typedef Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>::Index size_type;

		@@ -24,8 +24,9 @@ return_type_t<T_y, T_loc, T_scale, T_inv_scale> exp_mod_normal_lpdf(
		typedef partials_return_type_t<T_y, T_loc, T_scale, T_inv_scale>

		@@ -21,8 +21,9 @@ return_type_t<T_y, T_loc, T_scale, T_shape> pareto_type_2_cdf(
		typedef partials_return_type_t<T_y, T_loc, T_scale, T_shape>

		@@ -22,8 +22,9 @@ return_type_t<T_y, T_loc, T_scale, T_shape> pareto_type_2_lcdf(
		typedef partials_return_type_t<T_y, T_loc, T_scale, T_shape>

		@@ -14,37 +14,37 @@ typedef Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>::Index size_type;
		* The type of a matrix holding <code>var</code>

Refactor/clang tidy cleanup #1340

Refactor/clang tidy cleanup #1340

Conversation

SteveBronder commented Sep 5, 2019

Summary

Tests

Checklist

stan-buildbot commented Sep 6, 2019

rok-cesnovar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveBronder commented Sep 8, 2019

stan-buildbot commented Sep 9, 2019

rok-cesnovar left a comment

Choose a reason for hiding this comment